[MBL-1441] Remove 'Report This Project' Feature Flag #2063
Conversation
scottkicks
force-pushed
the
scott/debt/cleanup-report-project-flag
branch
from
b746052
to
e5f184b
Compare
scottkicks
requested review from
a team and
amy-at-kickstarter
and removed request for
a team
|
@scottkicks I thiiiink I see the difference in the screenshots - there's an extremely, extremely small change in the spacing in some of the buttons. It's barely visible, though. Any chance there's an Xcode/iOS SDK version difference that triggered the discrepancy? |
|
@amy-at-kickstarter, I don't think so. Removing this flag does mean that the Project page's DataSource technically has a new item in it (Report This Project row), so maybe that's adding space here 🤔 |
There was a problem hiding this comment.
LGTM, two non-blocking suggestions:
- Update your commit message for the large screenshot update to be more descriptive (just in case someone is investigating these tests in the future, and wondering about the huge update)
- Delete any screenshots that were made defunct by swapping to
orthogonalCombos
| @@ -719,7 +719,7 @@ final class ProjectPageViewControllerDataSourceTests: XCTestCase { | |||
| refTag: nil, | |||
| isExpandedStates: nil | |||
| ) | |||
| XCTAssertEqual(3, self.dataSource.numberOfSections(in: self.tableView)) | |||
| XCTAssertEqual(4, self.dataSource.numberOfSections(in: self.tableView)) | |||
There was a problem hiding this comment.
I assume the extra section is the report section?
| @@ -106,7 +106,7 @@ internal final class ProjectPageViewControllerTests: TestCase { | |||
| fetchProjectRewardsResult: .success([reward]) | |||
| ) | |||
|
|
|||
| combos(Language.allLanguages, [Device.phone4inch, Device.pad]).forEach { language, device in | |||
| orthogonalCombos(Language.allLanguages, [Device.phone4inch, Device.pad]).forEach { language, device in | |||
There was a problem hiding this comment.
I noticed no screenshots were deleted - there should be some that are now unused/defunct, correct?
Hrm, yeah, maybe. Some kind of gremlins in the layout engine? It's definitely unfortunate but seems fairly harmless at least. I didn't realize all these updated pngs were just from the Project page, whew. |
… reason for so many tests needing to update
scottkicks
force-pushed
the
scott/debt/cleanup-report-project-flag
branch
from
e5f184b
to
f284d8f
Compare
|
📲 What
Removes Report This Project Feature Flag.
🤔 Why
It has been on at 100% for some time now. It's time to say goodbye.
🛠 How
RemoteConfigFeatureand all subsequent tests and helpers👀 See
♿️ Accessibility
No changes
🏎 Performance
Same
✅ Acceptance criteria
⏰ TODO